Add argument for repeating transformations to optimize_for_target_gateset #6426
Conversation
NoureldinYosri
requested review from
vtomole,
cduck and
a team
as code owners
| circuit = circuits.Circuit( | ||
| op for op in circuit.all_operations() | ||
| ) # Ensure the circuit is contracted. |
There was a problem hiding this comment.
This leads to a loss of moment structure. When we wrote this function originally, preserving moment structure was an important requirement. Are you sure we want to do this?
There was a problem hiding this comment.
what was the original requirement exactly? the circuit in the issue is supposed to endup with 3 moments as per @eliottrosenberg which can't happen without some sort of contraction. probably there is a way to do the contraction without destroying the moment structure. but we need to clarify what it's exactly that we want to preserve or if we simply need another preserve_moment_structure parameter
There was a problem hiding this comment.
Yeah, I think reducing the number of moments is generally a good thing. In general, this can't preserve the number of moments (i.e. if I ask it to transform a cirq.ISWAP gate into CZ).
There was a problem hiding this comment.
@tanujkhattar apart from this line. the current implementation doesn't preserve the moment structure, for example look at the output in the issue. CZ(4, 5) is earlier than it should and PhXZ(a=0,x=1,z=0)(6)───PhXZ(a=0.5,x=-0.5,z=1)(6)-CZ(5, 6) are a lot later than they should.
There was a problem hiding this comment.
Let's continue the discussion on the original issue here - #6422 (comment)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6426 +/- ##
=======================================
Coverage 97.81% 97.81%
=======================================
Files 1111 1111
Lines 97143 97161 +18
=======================================
+ Hits 95022 95040 +18
Misses 2121 2121 ☔ View full report in Codecov by Sentry. |
|
@tanujkhattar now this PR just introduces the |
|
I've reviewed the other one, maybe we can directly merge that once my comments are addressed ? |
|
superseded by #6433 |
related to #6422